[PM-16102] Increase clickable area for bit-item actions #12450

vleague2 · 2024-12-17T20:48:16Z

🎟️ Tracking

📔 Objective

This PR increases the clickable area for each trailing bit-item action button so that the buttons are more accessible. To do so, we had to make a modification to the bit-badge (:before does not work when you have overflow:hidden, so we needed to move that property to an interior layer of badge), which necessitated changing it from a directive to a component. This should not affect any usages of the badge.

📸 Screenshots

Before:

Screen.Recording.2024-12-18.at.10.07.23.AM.mov

After:

Screen.Recording.2024-12-17.at.2.38.18.PM.mov

⏰ Reminders before review

Contributor guidelines followed
All formatters and local linters executed and passed
Written new unit and / or integration tests where applicable
Protected functional changes with optionality (feature flags)
Used internationalization (i18n) for all UI strings
CI builds passed
Communicated to DevOps any deployment requirements
Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 Reviewer guidelines

👍 (:+1:) or similar for great changes
📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
❓ (:question:) for questions
🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
🎨 (:art:) for suggestions / improvements
❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
⛏ (:pick:) for minor or nitpick changes

github-actions · 2024-12-17T20:57:06Z

Checkmarx One – Scan Summary & Details – 06574440-0de6-4588-ace0-afcc34584fc3

New Issues

Issue	Source File / Package	Checkmarx Insight
Client_Privacy_Violation	/libs/vault/src/cipher-view/item-history/item-history-v2.component.ts: 34	Attack Vector
Client_Privacy_Violation	/libs/vault/src/cipher-view/item-history/item-history-v2.component.ts: 34	Attack Vector
Client_Privacy_Violation	/libs/vault/src/cipher-view/item-history/item-history-v2.component.ts: 34	Attack Vector

Fixed Issues

Severity	Issue	Source File / Package
	Client_Privacy_Violation	/libs/tools/generator/components/src/credential-generator-history.component.html: 3
	Client_Privacy_Violation	/libs/tools/generator/components/src/username-generator.component.html: 3
	Client_Privacy_Violation	/apps/web/src/app/tools/password-generator-history.component.html: 11
	Client_Privacy_Violation	/libs/vault/src/components/password-history-view/password-history-view.component.html: 6
	Client_Privacy_Violation	/libs/vault/src/components/password-history-view/password-history-view.component.html: 6
	Client_Privacy_Violation	/apps/web/src/app/tools/password-generator-history.component.html: 11
	Client_Privacy_Violation	/libs/components/src/color-password/color-password.component.ts: 31

codecov · 2024-12-17T20:59:21Z

Codecov Report

Attention: Patch coverage is 71.42857% with 2 lines in your changes missing coverage. Please review.

Project coverage is 33.59%. Comparing base (0f9e18b) to head (7cf39ff).
Report is 20 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
libs/components/src/badge/badge.component.ts	75.00%	1 Missing ⚠️
libs/components/src/badge/badge.stories.ts	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #12450      +/-   ##
==========================================
+ Coverage   33.56%   33.59%   +0.02%     
==========================================
  Files        2926     2926              
  Lines       91527    91443      -84     
  Branches    17395    17376      -19     
==========================================
- Hits        30721    30717       -4     
+ Misses      58392    58311      -81     
- Partials     2414     2415       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

vleague2 · 2024-12-18T15:22:17Z

libs/components/src/item/item-action.component.ts

@@ -8,5 +8,9 @@ import { A11yCellDirective } from "../a11y/a11y-cell.directive";
  imports: [],
  template: `<ng-content></ng-content>`,
  providers: [{ provide: A11yCellDirective, useExisting: ItemActionComponent }],
+  host: {
+    class:
+      "[&>button]:tw-relative [&>button:not([bit-item-content])]:before:tw-content-[''] [&>button]:before:tw-absolute [&>button]:before:tw-block [&>button]:before:tw-top-[-0.75rem] [&>button]:before:tw-bottom-[-0.75rem] [&>button]:before:tw-right-[-0.25rem] [&>button]:before:tw-left-[-0.25rem]",


ℹ️ The not([bit-item-content]) is to prevent the :before area being added to the main item action, which also uses the item-action component.

willmartian · 2024-12-18T17:41:09Z

libs/components/src/item/item-action.component.ts

@@ -8,5 +8,9 @@ import { A11yCellDirective } from "../a11y/a11y-cell.directive";
  imports: [],
  template: `<ng-content></ng-content>`,
  providers: [{ provide: A11yCellDirective, useExisting: ItemActionComponent }],
+  host: {
+    class:
+      "[&>button]:tw-relative [&>button:not([bit-item-content])]:before:tw-content-[''] [&>button]:before:tw-absolute [&>button]:before:tw-block [&>button]:before:tw-top-[-0.75rem] [&>button]:before:tw-bottom-[-0.75rem] [&>button]:before:tw-right-[-0.25rem] [&>button]:before:tw-left-[-0.25rem]",


❓

In bit-item-content, we define the Y padding as tw-py-2 bit-compact:tw-py-1.5. The padding hooks into compact mode. My understanding is that the position properties here should be 1:1 with the padding, right? p-1.5 === .375rem and p-2 === .5rem

And if these values are meant to be coupled, would it be helpful to represent them as a shared CSS variable? --bit-item-x-padding etc? Then we could use the same variable in both places? Just wondering if we are worried about accidentally breaking this in the future if it is meant to be lockstep, but not actually lockstep.

Hm yeah it probably should be. I think I was eyeballing whether or not it looked like the right height and never went back and updated it

For posterity, they don't match because the padding units have a different origin point than the positioning units, but they are updated to be more accurate and responsive to compact mode in this commit: fc4c0eb

danielleflinn

✅ In storybook, verified bit-item actions click area now includes to top/bottom padding on the bit-item

danielleflinn

Whoops–was a bit fast with that approval...it would be nice if we could make this focus ring not be cropped by the edge of the bit-item. Is that possible?

willmartian · 2024-12-20T17:10:43Z

libs/components/src/badge/badge.component.ts

+@Component({
  selector: "span[bitBadge], a[bitBadge], button[bitBadge]",
-  providers: [{ provide: FocusableElement, useExisting: BadgeDirective }],
+  providers: [{ provide: FocusableElement, useExisting: BadgeComponent }],
+  imports: [CommonModule],
+  templateUrl: "badge.component.html",
  standalone: true,
 })
-export class BadgeDirective implements FocusableElement {
+export class BadgeComponent implements FocusableElement {


Just an FYI, this is a breaking change. Since an element can only have one component attached, it was possible to do <span bitBadge my-other-component> before this PR. After this PR, it would error.

Also, the Angular style guide suggests kebab-case for components, and pascalCase for directives. However, we are not consistently following this, and I don't think it is worth the code diff as part of this PR.

I'll do a quick check for any bitBadge instances doing that before I merge.

willmartian

LGTM, ty

danielleflinn

Focus ring padding fixed! Thanks!

[PM-16102] Increase clickable area for bit-item actions

67bf5b4

vleague2 commented Dec 18, 2024

View reviewed changes

Merge branch 'main' into ds/pm-16102/item-action-click-space

67b5d95

vleague2 marked this pull request as ready for review December 18, 2024 15:30

vleague2 requested a review from a team as a code owner December 18, 2024 15:30

vleague2 requested review from merissaacosta and willmartian and removed request for merissaacosta December 18, 2024 15:30

willmartian reviewed Dec 18, 2024

View reviewed changes

vleague2 added 2 commits December 19, 2024 13:48

update :before height and update story

fc4c0eb

Merge branch 'main' into ds/pm-16102/item-action-click-space

dfc5952

vleague2 requested a review from willmartian December 19, 2024 18:52

willmartian requested a review from danielleflinn December 19, 2024 22:00

danielleflinn approved these changes Dec 19, 2024

View reviewed changes

danielleflinn requested changes Dec 19, 2024

View reviewed changes

avoid conflicting with icon button's :before focus ring

7cf39ff

willmartian requested a review from danielleflinn December 20, 2024 17:07

willmartian reviewed Dec 20, 2024

View reviewed changes

willmartian approved these changes Dec 20, 2024

View reviewed changes

danielleflinn approved these changes Dec 20, 2024

View reviewed changes

vleague2 merged commit 64166a9 into main Dec 20, 2024
71 checks passed

vleague2 deleted the ds/pm-16102/item-action-click-space branch December 20, 2024 19:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PM-16102] Increase clickable area for bit-item actions #12450

[PM-16102] Increase clickable area for bit-item actions #12450

vleague2 commented Dec 17, 2024 •

edited by jira bot

Loading

github-actions bot commented Dec 17, 2024 •

edited

Loading

codecov bot commented Dec 17, 2024 •

edited

Loading

vleague2 Dec 18, 2024

willmartian Dec 18, 2024

vleague2 Dec 18, 2024

vleague2 Dec 19, 2024 •

edited

Loading

danielleflinn left a comment

danielleflinn left a comment

willmartian Dec 20, 2024

willmartian Dec 20, 2024

vleague2 Dec 20, 2024

willmartian left a comment

danielleflinn left a comment

[PM-16102] Increase clickable area for bit-item actions #12450

[PM-16102] Increase clickable area for bit-item actions #12450

Conversation

vleague2 commented Dec 17, 2024 • edited by jira bot Loading

🎟️ Tracking

📔 Objective

📸 Screenshots

⏰ Reminders before review

🦮 Reviewer guidelines

github-actions bot commented Dec 17, 2024 • edited Loading

New Issues

Fixed Issues

codecov bot commented Dec 17, 2024 • edited Loading

Codecov Report

vleague2 Dec 18, 2024

Choose a reason for hiding this comment

willmartian Dec 18, 2024

Choose a reason for hiding this comment

vleague2 Dec 18, 2024

Choose a reason for hiding this comment

vleague2 Dec 19, 2024 • edited Loading

Choose a reason for hiding this comment

danielleflinn left a comment

Choose a reason for hiding this comment

danielleflinn left a comment

Choose a reason for hiding this comment

willmartian Dec 20, 2024

Choose a reason for hiding this comment

willmartian Dec 20, 2024

Choose a reason for hiding this comment

vleague2 Dec 20, 2024

Choose a reason for hiding this comment

willmartian left a comment

Choose a reason for hiding this comment

danielleflinn left a comment

Choose a reason for hiding this comment

vleague2 commented Dec 17, 2024 •

edited by jira bot

Loading

github-actions bot commented Dec 17, 2024 •

edited

Loading

codecov bot commented Dec 17, 2024 •

edited

Loading

vleague2 Dec 19, 2024 •

edited

Loading